-
Notifications
You must be signed in to change notification settings - Fork 75
Implicit char to string v2 #1420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ataFrame.convert()
…sing, failing when the returned type is Char or String
…ing parsing, failing when the returned type is Char or String
c0d17a6
to
3211749
Compare
…s a char converter is passed explicitly
…generally a better explanation of how convertTo works
f785179
to
6d8d40a
Compare
6d8d40a
to
a29f5c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me mostly clear, the great idea to provide PR not only with tests, but doc changes - it's really help to jump into a logic
docs/StardustDocs/topics/convert.md
Outdated
* `Instant` (kotlinx.datetime, kotlin.time, and java.time) | ||
* `enum` classes (by name) | ||
|
||
Note that converting between `Char` and `Int` is done by ASCII character code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make it as a Warning or NOTE - it's a very important fact
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, nothing to add!
As @zaleslaw mentioned, there should be more warnings (and I'd add it everywhere in docs) that now Char
-> Int
is converted in ASCII coding.
Fixes #998 and surpasses #999.
(Can either be for
Beta3 orBeta4, whatever suits us best)We again have three parts, but slightly modified from #999:
String
-fallback forChar
inconvert
We get
String
-fallback forChar
columns indf.convert().to<>()
... andcol.convertTo<X>()
. This means you can now do things like:Note: We do keep the old
convert
behavior ofChar
->Int
by ASCII code. This is in line with the JVM world. We have parsing now if you want theChar
'1'
to turn into theInt
1
.Converting from
String
columns can only result in enum instances (like above), value class instances, or callparse()
, so we follow the same behavior forChar
columns now.Char
parsingdf.parse()
now not only parses string columns, but also char ones (this can be changed if you don't agree, of course).We also gain:
Here I deviate from #999, as
because the parsing didn't do anything. However similar to
String
columns, there'stryParse()
which can return the same type as the input.convertTo<> { parser {} }
Char
supportFixes #998 by trying out all provided
String
converters ifChar
s are encountered without a converter.I also updated the docs and expanded upon
convertTo
a lot to prevent confusion, like what was experienced in #998.